Skip to content

Conversation

petertodd
Copy link
Contributor

@petertodd petertodd commented May 27, 2021

Forwarding clone_from to the inner value changes the observable behavior, as previously the inner value would not be dropped by the default implementation.

Frankly, this is a super-niche case, so #85176 is welcome to argue the behavior should be otherwise! But if we overrride it, IMO documenting the behavior would be good.

Example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c5d0856686fa850c1d7ee16891014efb

Forwarding `clone_from` to the inner value changes the observable
behavior, as previously the inner value would *not* be dropped by the
default implementation.
@rust-highfive
Copy link
Contributor

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 27, 2021
@nagisa
Copy link
Member

nagisa commented May 28, 2021

Could you add a test for this behaviour?

@petertodd
Copy link
Contributor Author

petertodd commented May 30, 2021 via email

@JohnCSimon
Copy link
Member

ping from triage:
@petertodd assigning back to you to address a reviewer's question...

@rustbot label: -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 14, 2021
@nagisa
Copy link
Member

nagisa commented Jun 14, 2021

The test can be written as either an unit test inside the crate that defines this datatype or as a run-pass ui test (src/test/ui).

@nagisa nagisa added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jun 14, 2021
@m-ou-se m-ou-se assigned m-ou-se and unassigned joshtriplett Jun 14, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jun 14, 2021

We should discuss this issue to decide what behaviour we want. I'll open an issue for it. Once that decision is made, we should add a test for that behaviour to make sure it stays that way.

But for now let's merge and backport this to avoid breakage.

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 14, 2021

📌 Commit 5b2076f has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 14, 2021
@m-ou-se m-ou-se added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 14, 2021
@bors
Copy link
Collaborator

bors commented Jun 14, 2021

⌛ Testing commit 5b2076f with merge 7510b0c...

@bors
Copy link
Collaborator

bors commented Jun 14, 2021

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 7510b0c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 14, 2021
@bors bors merged commit 7510b0c into rust-lang:master Jun 14, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 14, 2021
@Mark-Simulacrum
Copy link
Member

Note: not backporting to 1.53, as #85176 is a 1.54 PR, so 1.53 did not have the override to revert.

@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.55.0, 1.54.0 Jun 17, 2021
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 17, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 23, 2021
…ulacrum

[beta] Bootstrap from stable

This is the follow up to master/beta promotion, as well as the first round of backports:

* Revert "Allow specifying alignment for functions rust-lang#81234"
* Revert rust-lang#85176 addition of clone_from for ManuallyDrop rust-lang#85758
* rustdoc: revert deref recur to resume inclusion of impl ExtTrait<Local> for ExtType rust-lang#84867
*  [beta] Update cargo rust-lang#86563

r? `@Mark-Simulacrum`
@petertodd petertodd deleted the 2021-revert-manuallydrop-clone-from branch July 17, 2021 01:33
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…one-from, r=m-ou-se

Revert rust-lang#85176 addition of `clone_from` for `ManuallyDrop`

Forwarding `clone_from` to the inner value changes the observable behavior, as previously the inner value would *not* be dropped by the default implementation.

Frankly, this is a super-niche case, so rust-lang#85176 is welcome to argue the behavior should be otherwise! But if we overrride it, IMO documenting the behavior would be good.

Example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c5d0856686fa850c1d7ee16891014efb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants